-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Tweak withAttributes in Flow #1658
Conversation
@pjfanning I think we should have this in 1.2.0-M1 , it's some kind of bug fix. |
Is there any way to add test coverage? Something like https://github.com/apache/pekko/blob/main/stream-tests/src/test/scala/org/apache/pekko/stream/scaladsl/FlowZipWithIndexSpec.scala#L60-L92 - which was added for #1525 |
Feel free to add it to m2, but actually the current fix not harm even without test. Otherwise ,it will cause problem when using in graphdsl as the zipwithIndex was implemented. |
I am uncomfortable with adding this in M1. There will be an M2 in a month or 2 and hopefully this can be added with some tests. |
aff004e
to
0e99b81
Compare
@pjfanning I have updated the code with test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Motivation:
To avoid #1525 , I checked all the
Flow#withAttributes
usage.Modification:
Move the
.withAttributes()
inside thevia(...)
Result: